Repository navigation
Conversation
Presently, `roll_forward`, `next_business_day`, et al. use
`day_interval_for`'s definition of `1` (days) for `Date` objects, or
`60 * 60 * 24` (seconds) otherwise.
Falling into the latter category, `ActiveSupport::TimeWithZone` objects
in Rails environments have their date component adjusted in a manner
which is not timezone-aware, and thus corrupts the time component when
the change spans a DST transition. For example:
t = TimeWithZone.new(Time.new(2026,10,23,12,0,0), ActiveSupport::TimeZone::country_zones(:gb)[0])
constructs `t` as noon UTC in `Europe/London` while BST is active, i.e.
`t` is 1pm local time.
next_business_day(t)
gives `2026-10-26 12:00:00.000000000 GMT +00:00`, that is, still noon
UTC, but now also noon local time, as the next business day was post
transition to GMT.
This commit addresses the issue by detecting `TimeWithZone`s, and using
ActiveSupport's `Integer#day` as for the interval in such cases; in the
above example giving `2026-10-26 13:00:00.000000000 GMT +00:00` as
expected.
loreenwong
reviewed
Nov 3, 2026
There was a problem hiding this comment.
Could we add a spec to test this across a DST boundary?
edit: Nevermind its here
loreenwong
approved these changes
Nov 3, 2026
Contributor
Author
|
Yeah, I didn't want to take on a dependency for it here, so test has limited utility but at least exercises the path. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Presently,
roll_forward,next_business_day, et al. useday_interval_for's definition of1(days) forDateobjects, or60 * 60 * 24(seconds) otherwise.Falling into the latter category,
ActiveSupport::TimeWithZoneobjects in Rails environments have their date component adjusted in a manner which is not timezone-aware, and thus corrupts the time component when the change spans a DST transition. For example:constructs
tas noon UTC inEurope/Londonwhile BST is active, i.e.tis 1pm local time.gives
2026-10-26 12:00:00.000000000 GMT +00:00, that is, still noon UTC, but now also noon local time, as the next business day was post transition to GMT.This commit addresses the issue by detecting
TimeWithZones, and using ActiveSupport'sInteger#dayas for the interval in such cases; in the above example giving2026-10-26 13:00:00.000000000 GMT +00:00as expected.